feat: add logout logic #38
Conversation
HackYourFutures
temporarily deployed
to
study-bridge-fe-user-lo-dtm7tr
There was a problem hiding this comment.
Pull request overview
Adds client-side logout support by introducing a logout API call plus React Query mutation/hook wrappers, integrating session cleanup and navigation on logout.
Changes:
- Added
logoutApi()to call the server logout endpoint. - Added
useLogoutMutation()to perform logout and clear client auth/query state. - Added a
useLogout()hook wrapper to exposelogout,isPending, anderror.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
client/src/hooks/useLogout.ts |
Provides a small hook wrapper around the logout mutation for component use. |
client/src/features/auth/mutations/useLogoutMutation.ts |
Implements the logout mutation and client-side cleanup (session/cache/nav). |
client/src/api/auth/auth.api.ts |
Adds the /api/auth/logout API call used by the mutation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onSuccess: async () => { | ||
| clearSession(); | ||
| localStorage.removeItem("hadSession"); | ||
| qc.clear(); | ||
| navigate("/", { replace: true }); | ||
| }, | ||
| onError: (error) => { | ||
| console.error("Logout failed:", error); | ||
| clearSession(); | ||
| localStorage.removeItem("hadSession"); | ||
| qc.clear(); | ||
| navigate("/", { replace: true }); | ||
| }, |
There was a problem hiding this comment.
onSuccess and onError contain the same logout cleanup logic (clear session, remove localStorage flag, clear query cache, navigate). This duplication increases the risk of future drift; consider extracting a shared helper or using onSettled to run the common cleanup once, while keeping error logging separate if needed.
| clearSession(); | ||
| localStorage.removeItem("hadSession"); | ||
| qc.clear(); | ||
| navigate("/", { replace: true }); | ||
| }, | ||
| onError: (error) => { | ||
| console.error("Logout failed:", error); | ||
| clearSession(); | ||
| localStorage.removeItem("hadSession"); | ||
| qc.clear(); | ||
| navigate("/", { replace: true }); |
There was a problem hiding this comment.
The app already has a centralized “hard logout” flow via triggerLogout/AuthBootstrap (used by the Axios interceptors). Implementing separate cleanup/navigation logic here can lead to inconsistent behavior across logout paths. Consider reusing the centralized handler (and ensuring it also removes hadSession) so all logout entrypoints share one implementation.
client/src/api/auth/auth.api.ts
Outdated
| export async function logoutApi() { | ||
| const res = await apiProtected.post("/api/auth/logout"); | ||
| return res.data; | ||
| } |
There was a problem hiding this comment.
logoutApi returns res.data with an implicit any type, but callers don’t use the return value. To keep API helpers consistent with register*Api (which just awaits the request) and improve typing, consider returning Promise<void> (or a specific response type if it’s meaningful) and avoid returning untyped data.
| export async function logoutApi() { | |
| const res = await apiProtected.post("/api/auth/logout"); | |
| return res.data; | |
| } | |
| export async function logoutApi(): Promise<void> { | |
| await apiProtected.post("/api/auth/logout"); | |
| } |
Dmytro-Doronin
left a comment
Dmytro-Doronin
left a comment
There was a problem hiding this comment.
Change everything that Copilot suggests you and call logOut api inside log out modal
client/src/hooks/useLogout.ts
Outdated
| @@ -0,0 +1,14 @@ | |||
| import { useLogoutMutation } from "../features/auth/mutations/useLogoutMutation"; | |||
|
|
|||
| export const useLogout = () => { | |||
There was a problem hiding this comment.
You do not need this hook, you can use useLogoutMutation directly in header component
3 participants
No description provided.